Skip to content

[Low Beverly] iP#107

Open
Bev-low wants to merge 60 commits into
nus-cs2113-AY2425S1:masterfrom
Bev-low:master
Open

[Low Beverly] iP#107
Bev-low wants to merge 60 commits into
nus-cs2113-AY2425S1:masterfrom
Bev-low:master

Conversation

@Bev-low

@Bev-low Bev-low commented Sep 3, 2024

Copy link
Copy Markdown

No description provided.

@Bev-low Bev-low changed the title IP [Low Beverly] IP Sep 3, 2024
@Bev-low Bev-low changed the title [Low Beverly] IP [Low Beverly] iP Sep 3, 2024

@TheDinos TheDinos left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, good job with minimal coding violations!

Comment thread src/main/java/ChattyCharlie.java Outdated

String farewell = "All the best in clearing your list!";

System.out.println(logo + charlie+ greeting);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the spacing be consistent?

Comment thread src/main/java/ChattyCharlie.java Outdated
if (index >= 0 && index < size) {
list[index].markTask();
int remainingTask = countUnmarkedTasks();
System.out.println(" Well Done! 1 task down, " + remainingTask + " to go.");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the space identation could be made into a constant variable given its usage throughout the code.

Comment thread src/main/java/ChattyCharlie.java Outdated
}

//To print list
public void toPrintList() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the function name could be clearer without using "to"?

Comment thread src/main/java/ChattyCharlie.java Outdated
} else if (line.startsWith("mark ")) {
//trim to get the task name
String taskDescription = line.substring(5).trim();
boolean found = false;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the boolean variable could be named "isFound" for greater clarity?

Comment thread src/main/java/ChattyCharlie.java Outdated
} else if (line.startsWith("mark ")) {
//trim to get the task name
String taskDescription = line.substring(5).trim();
boolean found = false;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Boolean is not in form of a question

Comment thread src/main/java/ChattyCharlie.java Outdated

//loop to find
for(int index = 0; index < toDo.size; index++)
{

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not following loop layout coding standard

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I dont understand what you mean, is this not the standard?

There are curly brackets, can I clarfiy what fo you mean

Comment thread src/main/java/ChattyCharlie.java Outdated

//loop to find
for(int index = 0; index < toDo.size; index++)
{

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, not following the loop layout coding standard

Comment thread src/main/java/ChattyCharlie.java Outdated
//LIST CLASS
public static class List {
//make a list of task
private Task[] list;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could name the array something in plural, like tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants